Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(weave): Update ToggleButtonGroup to allow disabling individual options #3194

Merged
merged 2 commits into from
Dec 11, 2024

Conversation

ericakdiaz
Copy link
Contributor

Description

Fixes nested button error in console, and fixes disabling individual options.

Testing

Tested on historic usage chart here https://github.com/wandb/core/pull/26192

@ericakdiaz ericakdiaz requested review from a team as code owners December 10, 2024 22:10
if (newValue !== value) {
if (
newValue !== value &&
options.find(option => option.value === newValue)?.isDisabled !== true
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'pointer-events-none' didn't work to prevent the onChange from firing, so moved the isDisabled check here, and fixed it below to not show a pointer cursor

@circle-job-mirror
Copy link

circle-job-mirror bot commented Dec 10, 2024

key={optionValue}
value={optionValue}
disabled={isDisabled}
asChild>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

asChild is needed to prevent a validateDomNesting error for nested buttons, see https://www.radix-ui.com/primitives/docs/guides/composition

@@ -9,6 +9,7 @@ import {Tailwind} from './Tailwind';
export type ToggleOption = {
value: string;
icon?: IconName;
isDisabled?: boolean;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added isDisabled prop to specific options

(icon
? 'gap-6 px-10 py-8 text-base'
: 'px-12 py-8 text-base'),
(isDisabled || optionIsDisabled) && 'cursor-auto opacity-35',
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

checks optionIsDisabled and changes pointer-events-none to cursor-auto

@ericakdiaz ericakdiaz merged commit f967984 into master Dec 11, 2024
120 checks passed
@ericakdiaz ericakdiaz deleted the ericakdiaz/fix-toggle-group-button branch December 11, 2024 00:23
@github-actions github-actions bot locked and limited conversation to collaborators Dec 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants